Skip to content

fix: reject passwords longer than 72 bytes#449

Merged
VaibhavAcharya merged 8 commits into
masterfrom
vaibhavacharya/ex-1862-identity-no-password-length-maximum-bcrypt-72-byte
May 28, 2026
Merged

fix: reject passwords longer than 72 bytes#449
VaibhavAcharya merged 8 commits into
masterfrom
vaibhavacharya/ex-1862-identity-no-password-length-maximum-bcrypt-72-byte

Conversation

@VaibhavAcharya

@VaibhavAcharya VaibhavAcharya commented May 26, 2026

Copy link
Copy Markdown
Contributor

- Summary

bcrypt.GenerateFromPassword silently truncates its input at 72 bytes, so a 200-character password only had its first 72 bytes verified on sign-in. This rejects passwords longer than 72 bytes at the API layer with a 422, and at the model layer (ErrPasswordTooLong) as defense in depth for non-HTTP callers like the CLI admin tool. The error message says "bytes" on purpose since CJK characters and emoji are multi-byte in UTF-8, and existing users with previously-truncated stored hashes keep signing in fine because bcrypt.CompareHashAndPassword also truncates.

- Test plan

- Description for the changelog

Reject passwords longer than 72 bytes (the bcrypt input limit) with a 422.

- A picture of a cute animal (not mandatory but encouraged)

@coderabbitai

coderabbitai Bot commented May 26, 2026 •
edited
Loading

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: cacc83cf-0cb8-4cba-8587-54f4dd4a46ec

📥 Commits

Reviewing files that changed from the base of the PR and between a04c6d7 and 7efa404.

📒 Files selected for processing (8)
  • api/admin.go
  • api/password.go
  • api/password_test.go
  • api/signup.go
  • api/user.go
  • api/verify.go
  • models/user.go
  • models/user_password_test.go
📝 Walkthrough

Summary by CodeRabbit

  • New Features
    • Added password length validation enforced across all user authentication and account management endpoints. Passwords must not exceed 72 bytes; any attempt to set a longer password will be rejected with a clear validation error message. This validation consistently applies to signup, password changes, email verification, and all administrative user management operations.

Walkthrough

This PR implements server-side password length validation across GoTrue's API endpoints. It introduces a 72-byte maximum password length limit at the model layer through a new MaxPasswordLength constant and ErrPasswordTooLong error. The hashPassword function now rejects passwords exceeding this limit. An API-layer validatePassword helper wraps this check and returns HTTP 422 responses for oversized passwords. This validation is applied to five endpoints: Signup, UserUpdate, Verify, adminUserUpdate, and adminUserCreate. Comprehensive tests cover both model-layer hashing and API-layer validation, including boundary cases and Unicode/emoji password handling.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fix: reject passwords longer than 72 bytes' accurately and clearly describes the main change in the pull request, which is adding validation to reject passwords exceeding bcrypt's 72-byte limit.
Description check ✅ Passed The description provides detailed context about the bcrypt truncation issue, explains the fix at both API and model layers, addresses edge cases, and includes a changelog entry—all directly related to the changeset.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch vaibhavacharya/ex-1862-identity-no-password-length-maximum-bcrypt-72-byte

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.12.2)

Error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions
The command is terminated due to an error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions

Comment @coderabbitai help to get the list of available commands and usage tips.

@VaibhavAcharya VaibhavAcharya marked this pull request as ready for review May 26, 2026 11:32
@VaibhavAcharya VaibhavAcharya requested a review from a team as a code owner May 26, 2026 11:32
@VaibhavAcharya VaibhavAcharya enabled auto-merge (squash) May 28, 2026 07:51
@VaibhavAcharya VaibhavAcharya merged commit aac5b57 into master May 28, 2026
7 checks passed
@VaibhavAcharya VaibhavAcharya deleted the vaibhavacharya/ex-1862-identity-no-password-length-maximum-bcrypt-72-byte branch May 28, 2026 08:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@aitchiss aitchiss aitchiss approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@VaibhavAcharya @aitchiss